Install typing_extensions if needed#98
Merged
aphedges merged 5 commits intoSHADE-AI:mainfrom Jan 28, 2025
Merged
Conversation
I copied multiple improvements from `chiron-utils`'s GitHub Actions setup to this file: - Enable running on pushes, which makes testing CI changes easier - Remove all GitHub API permissions for security - Cancel running CI job if a change is pushed - Update used actions to non-deprecated versions - Factor out Python version for easier reuse - Use stricter configuration for shell to catch more bugs Improvements copied from https://github.com/ALLAN-DIP/chiron-utils/blob/3a795038e73a7099dd9012c2480df7d35f74e05e/.github/workflows/checks.yaml
CI was broken because `ubuntu-latest` changed from Ubuntu 22.04 to 24.04, and the latter doesn't have Python 3.7. I pinned to using Ubuntu 22.04 so we can continue using Python 3.7. In addition, I pinned to the specific Python 3.7 patch release to limit possible breakage.
This commit contains multiple related changes:
- Update `dev` dependencies as much as possible
- The pinned versions did not work with Python 3.11, but using the
latest versions does not work with Python 3.7. The new versions work
for both Python 3.7 and 3.11.
- Pin `black` and `isort` versions
- Unlike the other `dev` dependencies, which are specified with `==`,
these two dependencies were specified with `>=`. I pinned these to
specific versions to match the other dependencies and limit
breakage.
- Update `pip` in `test-pr.yml`
- The version of `pip` shipped in GitHub Actions shows warnings that
are fixed by upgrading to the latest supported version.
The `typing_extensions` module was imported but not listed as a requirement, so importing `daidepp` failed if `typing_extensions` wasn't installed for another reason. I have added it as an explicit requirement and only use it if `Literal` or `get_args` cannot be imported directly from the `typing` module.
aphedges
force-pushed
the
install-typing_extensions
branch
from
9016ce6 to
8636323
Compare
Collaborator
Author
|
It turns out that the |
Collaborator
Author
|
@mjspeck, do you want to review this PR, or should I just go ahead and merge it? |
Collaborator
|
@aphedges reviewing now |
mjspeck
requested changes
Jan 28, 2025
Collaborator
Author
Thanks! I've made the requested changes. |
mjspeck
approved these changes
Jan 28, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I discovered a bug where importing from
typing_extensionswould fail because it's not listed as a dependency. Until now, I've always hadtyping_extensionsinstalled as a dependency of another package, so it took a while to encounter this particular bug.As part of this PR, I fixed the test setup so I could verify my changes don't break anything.